Skip to content

feat: 3-layer TaxDiagnosticResult model + migrate TDS/ITC/GST-RCM guards (close #39)#45

Merged
Rahul Dass (rahuldass19) merged 2 commits into
mainfrom
feat/issue-39-tax-diagnostic-result
Jun 21, 2026
Merged

feat: 3-layer TaxDiagnosticResult model + migrate TDS/ITC/GST-RCM guards (close #39)#45
Rahul Dass (rahuldass19) merged 2 commits into
mainfrom
feat/issue-39-tax-diagnostic-result

Conversation

@rahuldass19

@rahuldass19 Rahul Dass (rahuldass19) commented Jun 21, 2026

Copy link
Copy Markdown
Member

Summary

Implements the 3-layer TaxDiagnosticResult model for QWED-Tax, following the same pattern as qwed-verification #204 but as an independent model (no cross-package dependency).

What's new

qwed_tax/diagnostics.py — Core model:

  • TaxDiagnosticStatus enum: VERIFIED / UNVERIFIABLE / BLOCKED (tri-state only)
  • TaxDiagnosticResult frozen dataclass with 3 layers:
    • Layer 1: agent_message: str — agent-safe, no statute/rule IDs/detection logic
    • Layer 2: developer_fields: dict — structured evidence (constraint_id, statute, jurisdiction, audit_trace, deduction, net_payable, etc.)
    • Layer 3: proof_ref: Optional[str] — sha256 hash of retained proof artifact
  • TaxAdvisoryCheck — non-proof-bearing analysis (advisory_only=True enforced)
  • compute_proof_ref() — deterministic sha256 hash of JSON-serialized evidence
  • Factory methods: verified(), unverifiable(), blocked()
  • Invariants enforced in __post_init__: VERIFIED requires proof_ref, non-VERIFIED rejects proof_ref, agent_message must be non-empty

qwed_tax/audit.py — Extended:

  • trace_proof_ref(trace) — binds a build_trace() output to a deterministic hash

Guard migrations (3 of 12 — the ones that already have audit_trace)

Guard Method Status
TDSGuard to_diagnostic() Converts deduction-required, below-threshold, unknown-service
InputCreditGuard to_diagnostic() Converts eligible, blocked, gift-threshold
GSTGuard to_diagnostic() Converts verified, mismatch (BLOCKED), computed-only (UNVERIFIABLE), unknown-service

Each guard gets a to_diagnostic() static method that converts its existing dict return to TaxDiagnosticResult. The legacy dict API is unchanged — to_diagnostic() is additive, not breaking.

Tests

  • tests/test_diagnostics.py — 32 tests:
    • Model invariants (11 tests): proof_ref contract, frozen, empty message, factories, properties, to_dict/from_dict
    • TaxAdvisoryCheck (3 tests): advisory_only enforcement, roundtrip, in result
    • compute_proof_ref (3 tests): deterministic, different evidence, non-serializable
    • trace_proof_ref (2 tests): matches compute_proof_ref, different traces
    • TDS migration (3 tests): deduction, below-threshold, unknown
    • ITC migration (3 tests): eligible, blocked, gift-threshold
    • GST-RCM migration (4 tests): verified, mismatch, computed-only, unknown
  • 220 tests total, all passing

Remaining 9 guards

The remaining 9 guards (CapitalGains, Classification, Speculation, Setoff, Crypto, Valuation, Remittance, PoEM, Withholding) will be migrated in follow-up PRs. This PR establishes the contract and migrates the 3 guards that already have audit_trace — the lowest-effort, highest-confidence migrations.

Closes #39

Summary by CodeRabbit

  • New Features

    • Added a 3-state tax verification diagnostic system (Verified, Unverifiable, Blocked) with proof reference support and advisory checks.
    • Introduced deterministic proof references derived from audit traces for consistent verification records.
    • Added conversion helpers to translate existing guard outputs into the new diagnostic format (GST, TDS, and ITC eligibility).
  • Tests

    • Added comprehensive tests covering diagnostic invariants, serialization, deterministic hashing, and legacy-to-diagnostic conversions.

@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

@coderabbitai

coderabbitai Bot commented Jun 21, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

Introduces qwed_tax/diagnostics.py defining a 3-layer TaxDiagnosticResult frozen dataclass with TaxDiagnosticStatus (VERIFIED/UNVERIFIABLE/BLOCKED), TaxAdvisoryCheck, and compute_proof_ref. Extends audit.py with trace_proof_ref. Adds to_diagnostic() static methods to TDSGuard, InputCreditGuard, and GSTGuard that convert legacy dict outputs into TaxDiagnosticResult. New test suite covers all components.

Changes

3-Layer Diagnostic Model and Guard Migration

Layer / File(s) Summary
Proof-ref hashing primitives and TaxDiagnosticResult contract
qwed_tax/audit.py, qwed_tax/diagnostics.py
audit.py gains hashlib/json imports and trace_proof_ref(trace) function to compute deterministic sha256: proof references. New diagnostics.py defines TaxDiagnosticStatus enum with VERIFIED, UNVERIFIABLE, BLOCKED; TaxAdvisoryCheck frozen dataclass enforcing advisory_only=True; compute_proof_ref(evidence) helper; and TaxDiagnosticResult frozen dataclass with __post_init__ invariants tying VERIFIED to non-null proof_ref and non-VERIFIED to proof_ref=None, plus to_dict/from_dict serialization, factory constructors, and __all__ export.
Guard to_diagnostic() adapters
qwed_tax/guards/tds_guard.py, qwed_tax/guards/indirect_tax_guard.py, qwed_tax/jurisdictions/india/guards/gst_guard.py
Each guard receives a new to_diagnostic(result) static method converting legacy dict output into TaxDiagnosticResult. TDSGuard and InputCreditGuard branch on verified to emit blocked or verified. GSTGuard additionally branches on computed_only to emit unverifiable, blocked, or verified, propagating constraint_id, audit_trace, liability, and statute/jurisdiction metadata into developer fields and proof artifacts.
Test suite
tests/test_diagnostics.py
New test suite covers TaxDiagnosticResult construction invariants, factory correctness, frozen/immutable behavior, to_dict/from_dict round-trips with validation rejection, TaxAdvisoryCheck serialization, compute_proof_ref and trace_proof_ref determinism and error handling, and guard migration tests for all three adapters validating status mapping, proof_ref enforcement, and constraint_id propagation across success/failure/unverifiable scenarios.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

  • QWED-AI/qwed-tax#37: Establishes the audit_trace structure in audit.py and ITC/TDS guards that this PR's trace_proof_ref and to_diagnostic() adapters consume.
  • QWED-AI/qwed-tax#41: Alters verified/audit_trace fields in TDSGuard/InputCreditGuard fail-closed handling, directly shaping the dict shape that to_diagnostic() maps from.

Poem

🐇 A hash for every verdict, a proof for every claim,
Three layers in the audit — no two facts the same.
sha256: prefixed, the evidence is sealed,
VERIFIED or BLOCKED, the status is revealed.
Frozen dataclasses hold the verdict fast,
What's computed once shall deterministically last! 🔐

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 33.33% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately summarizes the main change: implementing a 3-layer TaxDiagnosticResult model and migrating three tax guards (TDS/ITC/GST), directly addressing issue #39.
Linked Issues check ✅ Passed All primary objectives from issue #39 are met: 3-layer TaxDiagnosticResult model with status standardization, architectural extensions to audit.py, proof artifact retention, invariant enforcement, and phased migration of TDS/ITC/GST-RCM guards.
Out of Scope Changes check ✅ Passed All code changes are directly aligned with issue #39 requirements: diagnostics model, guard migrations, proof hashing, and supporting tests. No extraneous changes detected.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/issue-39-tax-diagnostic-result

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Comment thread qwed_tax/guards/tds_guard.py Fixed
Comment thread qwed_tax/guards/tds_guard.py Fixed
Comment thread qwed_tax/audit.py
@greptile-apps

greptile-apps Bot commented Jun 21, 2026

Copy link
Copy Markdown

Greptile Summary

This PR introduces the TaxDiagnosticResult 3-layer model (status + agent message + developer fields + proof_ref) and migrates three guards (TDS, ITC, GST-RCM) from raw Dict[str, Any] returns to the new structured type via additive to_diagnostic() static methods that leave the existing dict API untouched.

  • diagnostics.py defines a frozen dataclass with structurally enforced invariants: VERIFIED requires a non-None proof_ref, non-VERIFIED rejects it, and empty agent_message is rejected at construction time.
  • audit.py gains trace_proof_ref(), a SHA-256 wrapper over build_trace() output that mirrors compute_proof_ref() in diagnostics.py.
  • All three to_diagnostic() implementations correctly raise ValueError when a VERIFIED result arrives without an audit_trace, closing the empty-dict proof_ref hole identified in the previous review round.

Confidence Score: 5/5

Safe to merge — the additive API design means no existing call sites are broken, and all three to_diagnostic() paths are fail-closed for non-VERIFIED results.

The core model invariants are well-enforced by the frozen dataclass and post_init. The to_diagnostic() methods correctly guard against the empty-evidence proof_ref issue raised in the previous review round. The two remaining concerns are design-level observations: the GSTGuard mismatch mapping is intentional per the PR description, and the proof_ref format validation gap is a hardening opportunity rather than a runtime defect. Neither affects the fail-closed behavior of the non-VERIFIED paths.

qwed_tax/jurisdictions/india/guards/gst_guard.py — to_diagnostic() mismatch-to-BLOCKED mapping conflates two distinct failure modes; qwed_tax/diagnostics.py — proof_ref format is not structurally validated in post_init or from_dict.

Important Files Changed

Filename Overview
qwed_tax/diagnostics.py New 3-layer TaxDiagnosticResult model; well-structured with frozen dataclass, invariant enforcement, and factory methods. Minor gap: proof_ref format is not structurally validated beyond a truthiness check.
qwed_tax/audit.py Adds trace_proof_ref() — a thin, correct SHA-256 wrapper over build_trace() output; mirrors compute_proof_ref() in diagnostics.py and raises ValueError on non-serializable input.
qwed_tax/guards/tds_guard.py Adds to_diagnostic() migration helper; correctly raises ValueError when verified=True but audit_trace is absent, and uses evidence=audit_trace (not evidence=audit_trace or {}).
qwed_tax/guards/indirect_tax_guard.py Adds to_diagnostic() for ITC eligibility results; correct fail-closed handling for blocked/unknown categories; guards against VERIFIED with no audit_trace.
qwed_tax/jurisdictions/india/guards/gst_guard.py Adds to_diagnostic() for RCM results; correctly separates computed_only → UNVERIFIABLE, but maps verified-mismatch (a completed computation with a wrong claim) to BLOCKED, which contradicts the module's own BLOCKED definition.
tests/test_diagnostics.py 32 tests covering model invariants, advisory checks, proof_ref hashing, and guard migrations; test_verification_mode_mismatch_to_diagnostic asserts BLOCKED for mismatch, encoding the semantic issue into the test suite.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A["Guard method returns Dict[str, Any]"] --> B{"verified?"}
    B -- "False" --> C{"computed_only?\n(GST only)"}
    C -- "True" --> D["TaxDiagnosticResult.unverifiable()\nstatus=UNVERIFIABLE\nproof_ref=None"]
    C -- "False" --> E["TaxDiagnosticResult.blocked()\nstatus=BLOCKED\nproof_ref=None"]
    B -- "True" --> F{"audit_trace\npresent?"}
    F -- "No" --> G["raise ValueError\n'VERIFIED requires audit_trace'"]
    F -- "Yes" --> H["compute_proof_ref(audit_trace)\n→ sha256:digest"]
    H --> I["TaxDiagnosticResult.verified()\nstatus=VERIFIED\nproof_ref=sha256:..."]
    I --> J{"__post_init__ invariants"}
    J -- "pass" --> K["Return TaxDiagnosticResult\nis_authoritative=True"]
    D --> L["Return TaxDiagnosticResult\nis_authoritative=False"]
    E --> L
    style G fill:#ff6b6b,color:#fff
    style K fill:#51cf66,color:#fff
    style L fill:#ffa94d,color:#fff
Loading
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
flowchart TD
    A["Guard method returns Dict[str, Any]"] --> B{"verified?"}
    B -- "False" --> C{"computed_only?\n(GST only)"}
    C -- "True" --> D["TaxDiagnosticResult.unverifiable()\nstatus=UNVERIFIABLE\nproof_ref=None"]
    C -- "False" --> E["TaxDiagnosticResult.blocked()\nstatus=BLOCKED\nproof_ref=None"]
    B -- "True" --> F{"audit_trace\npresent?"}
    F -- "No" --> G["raise ValueError\n'VERIFIED requires audit_trace'"]
    F -- "Yes" --> H["compute_proof_ref(audit_trace)\n→ sha256:digest"]
    H --> I["TaxDiagnosticResult.verified()\nstatus=VERIFIED\nproof_ref=sha256:..."]
    I --> J{"__post_init__ invariants"}
    J -- "pass" --> K["Return TaxDiagnosticResult\nis_authoritative=True"]
    D --> L["Return TaxDiagnosticResult\nis_authoritative=False"]
    E --> L
    style G fill:#ff6b6b,color:#fff
    style K fill:#51cf66,color:#fff
    style L fill:#ffa94d,color:#fff
Loading

Reviews (2): Last reviewed commit: "fix: address CodeQL, Sentry, Greptile P1..." | Re-trigger Greptile

Comment thread qwed_tax/guards/tds_guard.py
Comment thread qwed_tax/guards/tds_guard.py Outdated

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@qwed_tax/diagnostics.py`:
- Around line 187-205: The `__post_init__` method assumes `agent_message` is a
string without type validation, which can cause uncontrolled `AttributeError` on
bad payloads instead of clear validation errors. Add explicit type checks in
`__post_init__` to verify that `agent_message` is a string and
`developer_fields` (if used) are of the expected type, raising controlled
`ValueError` messages with clear descriptions if types are incorrect.
Additionally, in the `from_dict` method (referenced in the "Also applies to"
comment), add validation to ensure the `status` field is converted to or
validated as a `TaxDiagnosticStatus` enum value before object construction,
preventing non-enum status values from being accepted.

In `@tests/test_diagnostics.py`:
- Around line 60-61: The pytest.raises block in the frozen dataclass test is
catching too broad an exception type. Replace Exception with FrozenInstanceError
to explicitly assert for the specific exception that should be raised when
attempting to mutate a frozen instance. This ensures the test is checking for
the expected behavior and won't mask unrelated failures or unexpected
exceptions.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 5d12e032-581e-4c45-867e-e33a76261505

📥 Commits

Reviewing files that changed from the base of the PR and between 95735fe and 168fd2b.

⛔ Files ignored due to path filters (1)
  • qwed_tax/__pycache__/models.cpython-311.pyc is excluded by !**/*.pyc
📒 Files selected for processing (6)
  • qwed_tax/audit.py
  • qwed_tax/diagnostics.py
  • qwed_tax/guards/indirect_tax_guard.py
  • qwed_tax/guards/tds_guard.py
  • qwed_tax/jurisdictions/india/guards/gst_guard.py
  • tests/test_diagnostics.py

Comment thread qwed_tax/diagnostics.py
Comment thread tests/test_diagnostics.py Outdated
@sonarqubecloud

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (1)
qwed_tax/guards/tds_guard.py (1)

85-124: 🧹 Nitpick | 🔵 Trivial | ⚡ Quick win

Consider extracting the common to_diagnostic() logic into a shared helper.

The to_diagnostic() implementation is identical across all three guards migrated in this PR (TDSGuard, InputCreditGuard, GSTGuard). Since these guards are being added together, consolidating the conversion logic now would prevent future divergence and reduce maintenance burden.

♻️ Suggested consolidation approach

Create a shared helper in qwed_tax/diagnostics.py:

def legacy_result_to_diagnostic(
    result: Dict[str, Any],
    unknown_constraint_id: str = "UNKNOWN",
) -> TaxDiagnosticResult:
    """Convert a legacy guard result dict to TaxDiagnosticResult.
    
    Backward-compatible migration helper for guards that produce audit_trace.
    """
    verified = result.get("verified", False)
    audit_trace = result.get("audit_trace")

    if not verified:
        return TaxDiagnosticResult.blocked(
            agent_message="Tax verification could not be completed.",
            developer_fields={
                "constraint_id": audit_trace["rule_id"] if audit_trace else unknown_constraint_id,
                "audit_trace": audit_trace,
                "error": result.get("error"),
                "deduction": result.get("deduction"),
                "net_payable": result.get("net_payable"),
            },
        )

    if audit_trace is None:
        raise ValueError(
            "VERIFIED result requires audit_trace — "
            "use UNVERIFIABLE if no evidence was established."
        )

    return TaxDiagnosticResult.verified(
        agent_message="Tax verification completed.",
        developer_fields={
            "constraint_id": audit_trace["rule_id"],
            "statute": audit_trace.get("statute"),
            "jurisdiction": audit_trace.get("jurisdiction"),
            "audit_trace": audit_trace,
            "deduction": result.get("deduction"),
            "net_payable": result.get("net_payable"),
        },
        evidence=audit_trace,
    )

Then replace each guard's to_diagnostic() with:

`@staticmethod`
def to_diagnostic(result: Dict[str, Any]) -> TaxDiagnosticResult:
    """Convert legacy result to TaxDiagnosticResult."""
    return legacy_result_to_diagnostic(result, unknown_constraint_id="TDS_UNKNOWN")
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@qwed_tax/guards/tds_guard.py` around lines 85 - 124, The to_diagnostic()
static method in TDSGuard contains identical logic that is duplicated across
InputCreditGuard and GSTGuard. Extract this common conversion logic into a
shared helper function called legacy_result_to_diagnostic() in
qwed_tax/diagnostics.py, parameterizing the unknown_constraint_id so each guard
can pass its own identifier (e.g., "TDS_UNKNOWN" for TDSGuard). Then replace the
entire to_diagnostic() method body in TDSGuard with a single call to this shared
helper, passing "TDS_UNKNOWN" as the constraint identifier argument.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@qwed_tax/guards/tds_guard.py`:
- Around line 85-124: The to_diagnostic() static method in TDSGuard contains
identical logic that is duplicated across InputCreditGuard and GSTGuard. Extract
this common conversion logic into a shared helper function called
legacy_result_to_diagnostic() in qwed_tax/diagnostics.py, parameterizing the
unknown_constraint_id so each guard can pass its own identifier (e.g.,
"TDS_UNKNOWN" for TDSGuard). Then replace the entire to_diagnostic() method body
in TDSGuard with a single call to this shared helper, passing "TDS_UNKNOWN" as
the constraint identifier argument.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 78f9a404-71f5-48d4-97aa-713dd94fbb87

📥 Commits

Reviewing files that changed from the base of the PR and between 168fd2b and a5227cc.

📒 Files selected for processing (6)
  • qwed_tax/audit.py
  • qwed_tax/diagnostics.py
  • qwed_tax/guards/indirect_tax_guard.py
  • qwed_tax/guards/tds_guard.py
  • qwed_tax/jurisdictions/india/guards/gst_guard.py
  • tests/test_diagnostics.py
🚧 Files skipped from review as they are similar to previous changes (4)
  • qwed_tax/jurisdictions/india/guards/gst_guard.py
  • qwed_tax/guards/indirect_tax_guard.py
  • qwed_tax/diagnostics.py
  • tests/test_diagnostics.py

@rahuldass19

Copy link
Copy Markdown
Member Author

Not fixing the three suggested by coderabbit to_diagnostic() methods look similar but each guard packs its own domain fields (deduction/net_payable, eligible_itc, is_rcm/liability). A shared helper would need **kwargs for the varying bits, which is less explicit and loses type safety. With only 3 guards, the duplication is small and the per-guard code is clearer to read than a generic one-size-fits-all function.

@rahuldass19 Rahul Dass (rahuldass19) merged commit dda926b into main Jun 21, 2026
23 checks passed
@mintlify

mintlify Bot commented Jun 21, 2026

Copy link
Copy Markdown

Docs PR opened: QWED-AI/docs#226

Added a changelog entry for QWED-Tax adopting the 3-layer DiagnosticResult contract for TDS, ITC, and GST-RCM guards.

@mintlify

mintlify Bot commented Jun 21, 2026

Copy link
Copy Markdown

Docs PR opened: QWED-AI/docs#227

Added a Structured diagnostics section to the tax guards page documenting the new TaxDiagnosticResult tri-state model and proof references.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Structured Verification Diagnostics — 3-Layer TaxDiagnosticResult Model

1 participant